Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for binary paths on macOS #127

Merged
merged 17 commits into from
Dec 5, 2024
Merged

Add support for binary paths on macOS #127

merged 17 commits into from
Dec 5, 2024

Conversation

eugenesvk
Copy link
Contributor

@eugenesvk eugenesvk commented Dec 4, 2024 •
edited
Loading

Adds a utf8 conversion step to allow creating valid Mac URLs for its file manager APIs as well as Finder

  • via a fast simd method if everything is fine
  • on fail replaces bytes in invalid utf8 sequences with %-encoded utf8 strings that are valid paths

fixes: #124

Test: the actual delete tests are DISABLED since they fails in CI and locally by default since modern APFS doesn't allow binary paths, but I've manually tested on a HFS usb drive that supports it.

Is there an easy way to create some virtual FS during testing so that the test works both locally and in CIs and doesn't require manual intervention?

Also for some reason got the same non-Mac unrelated clippy errors

@eugenesvk eugenesvk mentioned this pull request Dec 4, 2024
Open
@Byron
Copy link
Owner

Byron commented Dec 4, 2024

Is there an easy way to create some virtual FS during testing so that the test works both locally and in CIs and doesn't require manual intervention?

hdiutil can be used (example), and maybe there is a way to set the filesystem as well.

It's a bit messy as these filesystems of course affect the whole system, breaking isolation, but in practice it was no issue if the temporary filesystem is used to the fullest extent.

Also for some reason got the same non-Mac unrelated clippy errors

They probably happen as clippy got better over time, and this code wasn't touched in a while. Fixing them shouldn't be a problem though.

@eugenesvk
Copy link
Contributor Author 

    // Don't run on CI as it's too slow there, resource busy, it fails more often than it succeeds by now.

:) so I guess not

@Byron
Copy link
Owner

Byron commented Dec 4, 2024

Even if it has to be disabled on CI in the end, it's useful to have test coverage on MacOS when running it locally, without assuming particular drives to be plugged in.

@eugenesvk
dep: add percent encoding support
e58e92b
@eugenesvk
add from_utf8_lossy_pc
8481d3c 
%-encodes invalid bytes in otherwise valid utf8 strings
@eugenesvk
test: add for from_utf8_lossy_pc
dc7dca0
@eugenesvk
dep: add simdutf8 for fast utf8 validation
3978204
@eugenesvk
remove automatic panicky conversion of potentially binary paths into …
d7d2187 
…non-binary strings

Issue: Byron#124
@eugenesvk
convert delete_using_file_mgr to use binary Paths
1d18e7a
@eugenesvk
convert delete_using_finder to use binary Paths
c013b9a
@eugenesvk
dep: move macos deps behind macos cfg target
6fbad98
@eugenesvk
test: new delete illegal bytes
bfbc394 
Disabled since only works on older FS, but tested manually to work on a USB HFS drive
@eugenesvk
cargo fmt
ab5c49b
@eugenesvk
fix finder extra escaping
d7295e8
@eugenesvk
cargo fmt
5d17879
@eugenesvk
fix Finder path generation for AS
0359a4d 
use the same %-conversion
@eugenesvk
test: new delete illegal bytes via Finder
175d6f5 
Disabled since only works on older FS, but tested manually to work on a USB HFS drive
@eugenesvk
cargo fmt
e499a0e
@eugenesvk
clippy
b147384
@eugenesvk eugenesvk mentioned this pull request Dec 4, 2024
Open
@Byron Byron force-pushed the fr-mac-byte-panic branch from 16669a3 to 58ac14d Compare December 5, 2024 07:04
@Byron
various refactors
d23a591 
* avoid unused import (it's only used on Linux)
* make tests work locally (and possibly on CI) by creating temporary filesystems.
@Byron Byron force-pushed the fr-mac-byte-panic branch from 58ac14d to d23a591 Compare December 5, 2024 07:05
@Byron Byron merged commit 6f0b737 into Byron:master Dec 5, 2024
4 checks passed
@eugenesvk eugenesvk deleted the fr-mac-byte-panic branch December 5, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Panic when deleting file with a non-UTF-8 name on a Mac
2 participants
@eugenesvk @Byron